Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThis PR introduces a comprehensive billing and licensing system with support for seat-based subscriptions, trial management, and offline license keys. It includes database migrations for license tracking, frontend UI for license management and billing, integration with a Lighthouse service for sync/checkout/invoicing, refactored entitlements evaluation, and conversion of entitlement checks to async operations throughout the codebase. Changes
Sequence Diagram(s)sequenceDiagram
participant Browser
participant WebApp as Web App<br/>(authenticatedPage)
participant License as License<br/>Service
participant Prisma as Database
participant Lighthouse as Lighthouse<br/>Service
Browser->>WebApp: Request org settings
WebApp->>Prisma: Get org + license
WebApp->>License: Check entitlements<br/>(async)
License->>Prisma: Load license by orgId
Prisma-->>License: Return license
License-->>WebApp: Entitlements resolved
WebApp->>Prisma: Get recent invoices
Prisma-->>WebApp: Return data
WebApp->>Browser: Render license settings UI<br/>+ banners
Browser->>WebApp: Activate license code
WebApp->>Lighthouse: POST /ping + activation code
Lighthouse-->>WebApp: Return license + entitlements
WebApp->>Prisma: Create/update license record
Prisma-->>WebApp: Success
WebApp->>Lighthouse: POST /ping<br/>(sync license state)
Lighthouse-->>WebApp: Confirm sync
WebApp-->>Browser: Success, refresh page
sequenceDiagram
participant User as End User
participant App as Web App<br/>(BannerSlot)
participant Resolver as Banner<br/>Resolver
participant Prisma as Database
participant Lighthouse as Service Ping<br/>Cron
Lighthouse->>Prisma: Update license<br/>lastSyncAt on success
Prisma-->>Lighthouse: Confirm
User->>App: Load app (authenticated)
App->>Prisma: Get org + license<br/>+ offline license
Prisma-->>App: Return data
App->>Resolver: Resolve active banner<br/>(priority, audience, dismissal)
Resolver->>Resolver: Evaluate conditions:<br/>license expired,<br/>invoice past due,<br/>trial state,<br/>ping staleness,<br/>permission sync pending
Resolver-->>App: Active BannerDescriptor
App->>App: Render banner component<br/>(LicenseExpiredBanner,<br/>TrialBanner, etc.)
App-->>User: Display banner UI
User->>App: Click "Manage license"
App-->>User: Navigate to<br/>/settings/license
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested labels
Suggested reviewers
✨ Finishing Touches🧪 Generate unit tests (beta)
|
97bb793 to
c99b3d0
Compare
User was hitting a unique constraint on UserToOrg(orgId, userId) when redeeming an invite, because onCreateUser auto-joins new signups in self-serve mode and redeemInvite then tried to create the same row. Make the insert idempotent via upsert so the downstream AccountRequest and invite cleanup still runs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds cancelAt to the License model and the lighthouse ping schema, and renders "Cancels on <date>" on the current plan card when there's no upcoming renewal. Prefers "Next renewal" when Stripe still has an upcoming invoice — so subscriptions scheduled to end after the next billing cycle keep showing the renewal row. Also makes nextRenewalAt / nextRenewalAmount nullable to match the lighthouse response, and guards new Date() against null in servicePing. Adds a CLAUDE.md under the lighthouse feature folder pointing at the service repo so the two schemas stay in lockstep. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Renders a dedicated card for offline (SOURCEBOT_EE_LICENSE_KEY) licenses showing the license id, seat cap, and expiry. When an offline license is present, the page skips the online license lookup entirely to mirror the precedence in entitlements.ts. Also adds a header row with a mailto link to support and an "All plans" shortcut to the public pricing page. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Introduces a priority-ordered, single-slot banner system under (app)/components/banners/. A server-side resolver picks the highest-priority banner that matches the current context (role, license state, offline license, permission-sync status) and renders it through a shared BannerShell that handles per-day dismissal via cookies. Banners included: - License expired (everyone, non-dismissible, role-aware copy) - License expiry heads-up (owner, dismissible, 14d window, uses formatDistance for relative copy) - Invoice past due (owner, non-dismissible) - Permission sync pending (everyone, non-dismissible, migrated from the prior standalone component through BannerShell) Precedence mirrors entitlements.ts: offline license is the sole source of truth when present, so online billing state is ignored. Also splits getValidOfflineLicense into a decode-only path so getOfflineLicenseMetadata can surface expired licenses to the UI. Includes bannerResolver.test.ts covering priority, audience filtering, dismissal filtering, offline/online expiry rules, and permission sync. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds trialEnd to the ping response schema and installId / requestTrial to the checkout request schema so types match the lighthouse service. createCheckoutSession passes the instance's SOURCEBOT_INSTALL_ID and defaults requestTrial to false (the existing "upgrade" button is not a trial path). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds a trial banner to the owner-facing banner stack and the
surrounding plumbing:
- Schema: License.trialEnd, License.hasPaymentMethod, Org.trialUsedAt
(durable flag that survives license deactivation). Two migrations.
- servicePing persists trialEnd / hasPaymentMethod and flips
Org.trialUsedAt on first trial sync.
- Trial banner (owner, dismissible, priority 25): title uses
formatDistance ("Your trial ends in 10 days"); copy + action branch
on hasPaymentMethod. With-PM variant links to /settings/license;
no-PM variant opens the Stripe portal via a new
OpenBillingPortalButton (LoadingButton + createPortalSession).
- currentPlanCard gains a "Trial ends on" fallback column for the
trial-without-PM case (where nextRenewalAt is null).
- activationCodeCard accepts isTrialEligible and flips its checkout
button from "Purchase a license" to "Start a free trial" when the
org hasn't trialed yet, passing requestTrial through to the checkout
endpoint.
- Types mirror the new lighthouse fields (trialEnd, hasPaymentMethod)
and the checkout request additions (installId, requestTrial).
Side-trips to Stripe (portal, checkout) now append ?refresh=true so
the license resyncs on return; trial-checkout also appends
?trial_used=true so Org.trialUsedAt flips immediately (closes the UX
gap between checkout completion and activation-code entry). page.tsx
handles both params, preserves any other query params, and redirects
to a clean URL.
Also: fetchWithRetry now only retries 5xx, 408, and 429 — 4xx errors
(e.g. TRIAL_ALREADY_USED at 409) propagate immediately instead of
retrying pointlessly.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
syncWithLighthouse previously swallowed ping errors with a log and early
return, which masked real problems. Flip the model: on a lighthouse
ServiceError response, throw ServiceErrorException. The sew() middleware
already knows how to marshal that into an API response for user-initiated
paths.
Callers fall into two camps:
- Propagate (user-initiated): activateLicense and refreshLicense. The
existing try/catch in activateLicense now correctly rolls back the
license row when lighthouse rejects the activation code; refreshLicense
lets the throw propagate so the UI surfaces a toast.
- Swallow explicitly (background / side-effect): license page load, the
24h cron, user-approval, and signup paths all wrap with
`.catch(() => { /* ignore */ })`. These happen as a side effect of
other successful operations; a ping failure shouldn't block them.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 13
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
CHANGELOG.md (1)
8-13:⚠️ Potential issue | 🟡 MinorChangelog entry appears to under-describe the PR scope.
Based on the PR objectives, this PR introduces a substantial feature set (banner system, Lighthouse integration, trial/billing flows, seat-based subscriptions, offline license support,
SOURCEBOT_LIGHTHOUSE_URL,Org.trialUsedAt, etc.), but the only[Unreleased]entry covers the offline-license crash fix. Consider adding entries underAdded/Changedfor the user-facing/operator-facing additions (e.g.,[EE]banner system, trial/billing UI, Lighthouse service integration, new env vars), so downstream consumers don't miss them in the release notes.As per coding guidelines: "Every PR must include a follow-up commit adding an entry to CHANGELOG.md under [Unreleased]. ... Prefix enterprise-only features with
[EE]".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CHANGELOG.md` around lines 8 - 13, Update the CHANGELOG.md [Unreleased] section to reflect the full scope of the PR rather than only the offline-license fix: add entries under "Added" and "Changed" that mention the banner system (mark enterprise-only with [EE] per guidelines), Lighthouse integration and the new SOURCEBOT_LIGHTHOUSE_URL env var, trial/billing UI and flows (reference Org.trialUsedAt), seat-based subscriptions, and offline license support/behavior change; ensure each user-facing or operator-facing feature (e.g., banner system, Lighthouse service, trial/billing, seat subscriptions) is clearly listed and EE-only items are prefixed with [EE] so downstream consumers see them in release notes.packages/web/src/app/api/(server)/ee/user/route.ts (1)
115-133:⚠️ Potential issue | 🟡 MinorWrite the
user.deleteaudit after the delete succeeds.
createAudit({ action: "user.delete", ... })is emitted beforeprisma.user.delete(...). If the delete throws (e.g., FK constraint, DB error), the catch on line 145 rethrows but the audit record is already persisted, producing a false "user deleted" entry. Move thecreateAuditcall below the successfulprisma.user.deleteto match the pattern used by the other audits in this PR (e.g.,chat.deletedinpackages/web/src/features/chat/actions.ts).🛠️ Proposed fix
- await createAudit({ - action: "user.delete", - actor: { - id: currentUser.id, - type: "user" - }, - target: { - id: userId, - type: "user" - }, - orgId: org.id, - }); - // Delete the user (cascade will handle all related records) await prisma.user.delete({ where: { id: userId, }, }); + + await createAudit({ + action: "user.delete", + actor: { id: currentUser.id, type: "user" }, + target: { id: userId, type: "user" }, + orgId: org.id, + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/src/app/api/`(server)/ee/user/route.ts around lines 115 - 133, The audit entry for "user.delete" is being created before the actual deletion so failures produce false positives; move the createAudit call that constructs action: "user.delete" (currently using createAudit and referencing currentUser.id, userId, org.id) to after the successful await prisma.user.delete({ where: { id: userId } }) call so the audit is only persisted on success, matching the pattern used by chat.deleted in packages/web/src/features/chat/actions.ts; ensure you keep the same actor/target/org payload and error handling unchanged.packages/web/src/auth.ts (1)
260-281:⚠️ Potential issue | 🟡 Minor
getIssuerUrlForAccountre-resolves all providers on every JWT callback — amplified by the lazy migration loop.The
jwtcallback runs on every token refresh/verify. When a user has one or more accounts withoutissuerUrl(the lazy migration path),getIssuerUrlForAccountis invoked in a loop, and each invocation now callsawait getProviders()— which in turn callsawait hasEntitlement("sso")and potentiallyawait getEEIdentityProviders(). That's N (accounts) × entitlement-check + SSO-factory construction per token refresh.Consider hoisting a single
await getProviders()outside the loop, or caching the provider list at the module level (since provider configuration doesn't change per-request anyway).🔧 Proposed fix
if (token.userId) { const accountsWithoutIssuerUrl = await __unsafePrisma.account.findMany({ where: { userId: token.userId, issuerUrl: null, }, }); - for (const account of accountsWithoutIssuerUrl) { - const issuerUrl = await getIssuerUrlForAccount(account); + if (accountsWithoutIssuerUrl.length > 0) { + const providers = await getProviders(); + for (const account of accountsWithoutIssuerUrl) { + const issuerUrl = getIssuerUrlForAccountFromProviders(account, providers); ... + } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/src/auth.ts` around lines 260 - 281, The jwt callback's lazy migration loop calls getIssuerUrlForAccount for each account which internally calls getProviders() causing repeated entitlement checks; hoist a single await getProviders() before iterating accounts (or retrieve/cached providers at module level) and pass that provider list into getIssuerUrlForAccount (or add an overload/param) so the loop reuses the same providers instead of calling getProviders() N times; update references to getIssuerUrlForAccount and any callers in the jwt callback to accept and use the pre-fetched providers.packages/web/src/lib/authUtils.ts (1)
105-121:⚠️ Potential issue | 🟠 MajorTOCTOU race on
orgHasAvailability→userToOrg.createcan exceed the seat cap.
orgHasAvailability(defaultOrg.id)runs afindUniqueOrThrow(count-based) outside any transaction, then__unsafePrisma.userToOrg.createruns separately on Lines 114–120. Two concurrent sign-ups can both observememberCount < seatCap, then both insert, pushing the org above the cap. Same pattern inaddUserToOrganization(Lines 186–212): thehasAvailabilitycheck happens before the transaction and the transaction never re-checks under a lock. Consider moving the availability check and membership insert into a single$transaction(withSerializableisolation, or acountinside the transaction followed by the insert), or relying on a DB-level seat-count constraint.🛠️ Sketch of a fix (onCreateUser)
else if (!defaultOrg.memberApprovalRequired) { - // Don't exceed the licensed seat count. The user row still exists; - // they just aren't attached to the org until a seat frees up. - const hasAvailability = await orgHasAvailability(defaultOrg.id); - if (!hasAvailability) { - logger.warn(`onCreateUser: org ${SINGLE_TENANT_ORG_ID} has reached max capacity. User ${user.id} was not added to the org.`); - return; - } - - await __unsafePrisma.userToOrg.create({ - data: { - userId: user.id, - orgId: SINGLE_TENANT_ORG_ID, - role: OrgRole.MEMBER, - } - }); + // Don't exceed the licensed seat count. Availability + insert must be + // atomic to prevent TOCTOU over-allocation under concurrent signups. + const added = await __unsafePrisma.$transaction(async (tx) => { + const seatCap = getSeatCap(); + if (seatCap) { + const memberCount = await tx.userToOrg.count({ where: { orgId: defaultOrg.id } }); + if (memberCount >= seatCap) { + return false; + } + } + await tx.userToOrg.create({ + data: { userId: user.id!, orgId: SINGLE_TENANT_ORG_ID, role: OrgRole.MEMBER }, + }); + return true; + }, { isolationLevel: 'Serializable' }); + + if (!added) { + logger.warn(`onCreateUser: org ${SINGLE_TENANT_ORG_ID} has reached max capacity. User ${user.id} was not added to the org.`); + return; + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/src/lib/authUtils.ts` around lines 105 - 121, The org seat-check and membership create are vulnerable to a TOCTOU race: replace the separate orgHasAvailability(...) call followed by __unsafePrisma.userToOrg.create(...) in onCreateUser and the same pattern in addUserToOrganization with a single atomic operation (use a Prisma $transaction that does the count/check and the create inside the same transaction with Serializable isolation, or perform the count and insert inside the same transaction and re-check capacity before inserting), or alternatively enforce a DB-level seat-count constraint and handle the unique/constraint error on create; update code paths referencing orgHasAvailability, __unsafePrisma.userToOrg.create, onCreateUser, and addUserToOrganization to use the transactional approach and surface a clear error/log when capacity is exceeded.
🟡 Minor comments (12)
packages/web/src/ee/features/lighthouse/CLAUDE.md-9-11 (1)
9-11:⚠️ Potential issue | 🟡 MinorSpecify a language on the fenced code block (MD040).
markdownlint flags this fence as missing a language. Since the content is a path/route hint, use
text(or a placeholder likeplaintext) for consistency.📝 Proposed fix
-``` +```text lighthouse: lambda/routes/<route>.ts</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@packages/web/src/ee/features/lighthouse/CLAUDE.mdaround lines 9 - 11, The
fenced code block in CLAUDE.md is missing a language tag; update the
triple-backtick fence that contains the text "lighthouse:
lambda/routes/.ts" to include a language identifier (e.g., text or
plaintext) so the block becomestext ..., ensuring markdownlint MD040 is
satisfied.</details> </blockquote></details> <details> <summary>packages/db/prisma/migrations/20260417224042_remove_guest_org_role/migration.sql-12-21 (1)</summary><blockquote> `12-21`: _⚠️ Potential issue_ | _🟡 Minor_ **Explicit BEGIN/COMMIT inside a Prisma-wrapped migration — confirm this is intentional.** Prisma runs each migration inside its own transaction; adding an explicit `BEGIN;` / `COMMIT;` in the SQL will emit a Postgres warning (`there is already a transaction in progress`) and the inner `COMMIT` closes the outer transaction, so statements on lines 8–10 run *inside* the outer transaction while the enum swap effectively commits the whole thing mid-migration. This is the pattern Prisma's own enum-removal generator emits, so it's likely fine, but please double-check that the pre-transaction `DELETE`s (lines 9–10) behave as expected if the enum swap later fails — they will already be committed by line 21. <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@packages/db/prisma/migrations/20260417224042_remove_guest_org_role/migration.sql` around lines 12 - 21, The migration contains explicit BEGIN;/COMMIT; which conflicts with Prisma's transaction wrapper and causes the enum swap to commit mid-migration; remove the explicit BEGIN and COMMIT so the statements (CREATE TYPE "OrgRole_new", ALTER TABLE "UserToOrg" ALTER COLUMN "role" ..., ALTER TYPE ... RENAME, DROP TYPE, ALTER TABLE ... SET DEFAULT) all run inside Prisma's transaction, or alternatively move any pre-transaction DELETEs into the same transaction after the enum swap; update the SQL to drop the BEGIN/COMMIT wrappers (affecting the migration's CREATE TYPE "OrgRole_new", ALTER TABLE "UserToOrg" ALTER COLUMN "role", and ALTER TYPE "OrgRole" RENAME steps) so the migration is executed atomically by Prisma. ``` </details> </blockquote></details> <details> <summary>packages/web/src/app/(app)/components/banners/actions.ts-6-15 (1)</summary><blockquote> `6-15`: _⚠️ Potential issue_ | _🟡 Minor_ **Validate `id` at runtime before writing a cookie.** `BannerId` is a compile-time type only; server actions receive untrusted runtime input. As written, a caller can pass any string and set an arbitrary cookie name under the `DISMISS_COOKIE_PREFIX`. It's low-impact (scoped to the caller's own session and prefixed), but adding a runtime check against the known `BannerId` list (same one used in `bannerSlot.tsx`) avoids growing a surface that later readers may trust. <details> <summary>🛡️ Suggested guard</summary> ```diff export async function dismissBanner(id: BannerId) { + if (!KNOWN_BANNER_IDS.includes(id)) { + return; + } const cookieStore = await cookies(); ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@packages/web/src/app/`(app)/components/banners/actions.ts around lines 6 - 15, The dismissBanner server action accepts an untrusted runtime id (BannerId is compile-time only) and must validate it before writing a cookie; update dismissBanner to check the incoming id against the canonical list of valid banner ids (the same list used in bannerSlot.tsx) and return or throw on invalid values, then only call cookies() and cookieStore.set(`${DISMISS_COOKIE_PREFIX}${id}`, ...) for validated ids; reference the DISMISS_COOKIE_PREFIX constant and the dismissBanner function when applying the guard so the runtime validation mirrors the bannerSlot.tsx source of truth. ``` </details> </blockquote></details> <details> <summary>packages/web/src/app/(app)/components/banners/trialBanner.tsx-17-28 (1)</summary><blockquote> `17-28`: _⚠️ Potential issue_ | _🟡 Minor_ **Title reads awkwardly once the trial end date has passed.** `formatDistance(..., { addSuffix: true })` returns either `"in X days"` or `"X days ago"`. If `trialEnd` is in the past for any reason (e.g., the resolver hasn't yet recomputed, or there's a small clock skew between server-rendered `now` and actual `trialEnd`), the title renders as "Your trial ends 2 hours ago", which is grammatically wrong. Consider branching on whether `trialEndDate > now` and using a different phrase (e.g., "Your trial ended X ago") for the past case, or guarding the banner from rendering once `trialEnd` is in the past. <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@packages/web/src/app/`(app)/components/banners/trialBanner.tsx around lines 17 - 28, The title currently uses formatDistance(trialEndDate, now, { addSuffix: true }) which yields "in X" or "X ago" causing grammar like "Your trial ends X ago"; change logic in trialBanner.tsx to detect whether trialEndDate > now (e.g., const isFuture = trialEndDate > now) and then set the BannerShell title accordingly (e.g., title={isFuture ? `Your trial ends ${relative}` : `Your trial ended ${formatDistance(trialEndDate, now)}`) or alternatively skip rendering the BannerShell when the trial is already past; update references to relative, trialEndDate, now, and the BannerShell title prop to implement this branch. ``` </details> </blockquote></details> <details> <summary>packages/web/src/lib/utils.ts-602-621 (1)</summary><blockquote> `602-621`: _⚠️ Potential issue_ | _🟡 Minor_ **`fetchWithRetry` doesn't respect `AbortSignal` cancellation.** If the caller passes an `AbortSignal` via `init`, an abort during a `fetch` will throw an `AbortError` which is caught here and treated like any other transient failure — the function will sleep and retry until `retries` is exhausted, defeating the cancellation. Consider rethrowing immediately on abort: <details> <summary>🛠 Proposed fix</summary> ```diff } catch (error) { + if (init?.signal?.aborted || (error instanceof DOMException && error.name === 'AbortError')) { + throw error; + } if (attempt === retries) { throw error; } } ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@packages/web/src/lib/utils.ts` around lines 602 - 621, fetchWithRetry currently swallows AbortErrors and keeps retrying; update it to respect AbortSignal by (1) checking if init?.signal?.aborted before each attempt and immediately throwing that signal's AbortError, and (2) in the catch block for the fetch inside fetchWithRetry, rethrow immediately if the caught error is an AbortError (e.g., error.name === 'AbortError' or error instanceof DOMException with name 'AbortError') instead of treating it as a transient failure; keep the existing retry/backoff behavior for other errors and non-abort failures. ``` </details> </blockquote></details> <details> <summary>packages/web/src/auth.ts-297-297 (1)</summary><blockquote> `297-297`: _⚠️ Potential issue_ | _🟡 Minor_ **Top-level `await` at module init: Confirm tsconfig supports transpilation and address runtime entitlement limitation.** `providers: (await getProviders()).map(...)` uses top-level `await` in a module-level `NextAuth()` call. While your tsconfig targets ES2017 (which lacks native top-level await support), Next.js 16.2.3 with SWC handles transpilation automatically, so this works in practice. The real issue: `getProviders()` is called once at module initialization. Entitlement changes at runtime (e.g., SSO entitlement enabled via license update) won't reflect until the Next.js server restarts. If dynamic entitlement support is required, move provider resolution into a dynamic path (e.g., a route handler or server action) rather than caching at init. <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@packages/web/src/auth.ts` at line 297, The module calls getProviders() with a top-level await inside the NextAuth() configuration (providers: (await getProviders()).map(...)), causing provider resolution to happen once at module initialization and not reflect runtime entitlement changes; refactor so provider resolution occurs per-request by removing the top-level await and moving the getProviders() call into a dynamic handler (e.g., a route handler or server action) or into the request-time part of NextAuth configuration so getProviders() is invoked on each request (look for the NextAuth() call and the providers property and replace the static module-init mapping with an async per-request resolution). ``` </details> </blockquote></details> <details> <summary>packages/shared/src/entitlements.test.ts-208-216 (1)</summary><blockquote> `208-216`: _⚠️ Potential issue_ | _🟡 Minor_ **Minor: the "boundary" test isn't actually at the boundary.** `lastSyncAt` is `Date.now() - STALE_THRESHOLD_MS + 1000`, i.e. 1 second inside the threshold — not exactly on it. If the intent is to pin the `<=` semantics of the comparison, set it to `Date.now() - STALE_THRESHOLD_MS` (or add a second case at `- STALE_THRESHOLD_MS - 1` to show the flip). Otherwise the comment reads stronger than what the test actually asserts. <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@packages/shared/src/entitlements.test.ts` around lines 208 - 216, The test "returns entitlements at the threshold boundary" currently sets lastSyncAt to Date.now() - STALE_THRESHOLD_MS + 1000 which is inside the threshold; change it to exactly Date.now() - STALE_THRESHOLD_MS to exercise the boundary (or add a second assertion with Date.now() - STALE_THRESHOLD_MS - 1 to demonstrate the flip). Update the makeLicense call in this test (and/or add the second case) so getEntitlements is invoked with a license whose lastSyncAt equals the exact STALE_THRESHOLD_MS boundary to pin the <= semantics. ``` </details> </blockquote></details> <details> <summary>packages/web/src/app/(app)/settings/license/page.tsx-43-48 (1)</summary><blockquote> `43-48`: _⚠️ Potential issue_ | _🟡 Minor_ **Unsafe cast of `searchParams` when rebuilding the URL — arrays and `undefined` values will be mangled.** `searchParams` is typed as `Record<string, string | string[] | undefined>` (per `LicensePageProps` on Line 16), but it's cast to `Record<string, string>` before being handed to `URLSearchParams`. If any incoming param is an array, `URLSearchParams` will call `toString()` on it and serialize as `"a,b,c"` (not as repeated keys); `undefined` values become the literal string `"undefined"`. Safer to construct the `URLSearchParams` manually and skip non-strings. <details> <summary>🛠️ Proposed fix</summary> ```diff - // Strip our params but preserve anything else (e.g. `checkout=success`). - const preserved = new URLSearchParams(searchParams as Record<string, string>); - preserved.delete('refresh'); - preserved.delete('trial_used'); + // Strip our params but preserve anything else (e.g. `checkout=success`). + const preserved = new URLSearchParams(); + for (const [key, value] of Object.entries(searchParams ?? {})) { + if (key === 'refresh' || key === 'trial_used' || value === undefined) { + continue; + } + if (Array.isArray(value)) { + for (const v of value) { preserved.append(key, v); } + } else { + preserved.append(key, value); + } + } const suffix = preserved.toString(); redirect(suffix ? `/settings/license?${suffix}` : '/settings/license'); ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@packages/web/src/app/`(app)/settings/license/page.tsx around lines 43 - 48, The code unsafely casts searchParams to Record<string,string> before passing into URLSearchParams which will mangle arrays and undefineds; change the logic that builds preserved so you create a new URLSearchParams and iterate over Object.entries(searchParams) (the searchParams prop from LicensePageProps), skipping undefined, appending each string value directly and appending each element when the value is an array, then delete('refresh') and delete('trial_used') on that preserved URLSearchParams and call redirect(suffix ? `/settings/license?${suffix}` : '/settings/license') as before; update references to preserved, searchParams and redirect accordingly. ``` </details> </blockquote></details> <details> <summary>packages/web/src/app/(app)/components/banners/bannerResolver.tsx-165-194 (1)</summary><blockquote> `165-194`: _⚠️ Potential issue_ | _🟡 Minor_ **`expiring-soon` won't fire for auto-renewing online subscriptions.** Online heads-up currently depends on `ctx.license.cancelAt`. For a normal monthly/yearly subscription that auto-renews, Stripe leaves `cancel_at` null and only sets it once the user schedules cancellation — so paying users will never see the "License expires in N days" banner, only users who have already scheduled a cancel. The `nextRenewalAt` field exists in the License schema and is available in context. Consider also keying off `nextRenewalAt` to cover approaching auto-renewals (in addition to the scheduled cancellation case). <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@packages/web/src/app/`(app)/components/banners/bannerResolver.tsx around lines 165 - 194, getLicenseExpiryState currently only checks ctx.license.cancelAt for online "expiring-soon" state, so auto-renewing subscriptions (where cancelAt is null) never trigger the heads-up; update getLicenseExpiryState to also consider ctx.license.nextRenewalAt (parse to Date, compute deltaMs vs ctx.now.getTime()) and if deltaMs > 0 && deltaMs <= EXPIRY_HEADS_UP_WINDOW_MS return { kind: 'expiring-soon', source: 'online', expiresAt } just like the cancelAt branch, while preserving existing cancelAt and expired-status checks (ensure you reference getLicenseExpiryState, ctx.license.cancelAt, ctx.license.nextRenewalAt, EXPIRY_HEADS_UP_WINDOW_MS and return the same LicenseExpiryState shape). ``` </details> </blockquote></details> <details> <summary>packages/web/src/actions.ts-744-744 (1)</summary><blockquote> `744-744`: _⚠️ Potential issue_ | _🟡 Minor_ **Use `logger.error` for consistency with the rest of the file.** Every other error log in this file goes through the module-level `logger` (e.g. Line 49, Line 546, Line 579, Line 611). Dropping to `console.error` here bypasses the structured logger and likely breaks log-level filtering / JSON formatting in production. <details> <summary>Suggested change</summary> ```diff - console.error(`Anonymous access isn't supported in your current plan. For support, contact ${SOURCEBOT_SUPPORT_EMAIL}.`); + logger.error(`Anonymous access isn't supported in your current plan. For support, contact ${SOURCEBOT_SUPPORT_EMAIL}.`); ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@packages/web/src/actions.ts` at line 744, Replace the direct console.error call in the anonymous-access handling with the module-level logger by using logger.error instead of console.error so logs follow the file's structured logging conventions; locate the console.error invocation in packages/web/src/actions.ts (the anonymous access check) and change it to call logger.error with the same message (preserving SOURCEBOT_SUPPORT_EMAIL) so it uses the existing logger used elsewhere in this file. ``` </details> </blockquote></details> <details> <summary>packages/web/src/ee/features/lighthouse/actions.ts-40-49 (1)</summary><blockquote> `40-49`: _⚠️ Potential issue_ | _🟡 Minor_ **Rollback delete can mask the original sync error.** If `syncWithLighthouse` throws and then `prisma.license.delete` also throws (DB transient error, unique-key race, etc.), `e` is never re-thrown — the delete's error propagates instead and the operator loses the original failure reason (typically the more actionable one). Swallow/log the rollback failure so the sync error is preserved. <details> <summary>Suggested change</summary> ```diff try { await syncWithLighthouse(org.id); } catch (e) { - // If the ping fails, remove the license record - await prisma.license.delete({ - where: { orgId: org.id }, - }); - - throw e; + // If the ping fails, remove the license record. Don't let a + // rollback failure mask the original sync error. + try { + await prisma.license.delete({ + where: { orgId: org.id }, + }); + } catch (rollbackError) { + // log-and-continue so `e` is the error surfaced to the caller + // eslint-disable-next-line no-console + console.error('Failed to roll back license row after sync failure', rollbackError); + } + throw e; } ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@packages/web/src/ee/features/lighthouse/actions.ts` around lines 40 - 49, The current catch block around syncWithLighthouse(org.id) performs prisma.license.delete which can throw and replace the original error; change it to preserve and rethrow the original sync error: inside the catch(e) capture the original error (e.g., originalErr = e), then attempt the rollback delete in its own try/catch (call prisma.license.delete({ where: { orgId: org.id } }) inside a nested try), and if that nested delete fails, log the rollback failure (do not throw), and finally rethrow the originalErr; reference syncWithLighthouse, prisma.license.delete and org.id when locating the code to update. ``` </details> </blockquote></details> <details> <summary>packages/web/src/ee/features/lighthouse/actions.ts-190-214 (1)</summary><blockquote> `190-214`: _⚠️ Potential issue_ | _🟡 Minor_ **Bound the pagination loop to prevent runaway iteration on a misbehaving server.** `while (true)` with termination driven entirely by `result.hasMore` and a server-provided `lastInvoice.id` means a Lighthouse-side bug that keeps returning `hasMore: true` with a non-advancing cursor (or a duplicate id) will spin this server action indefinitely, holding the request thread open. A hard page cap is cheap defense-in-depth for a remote dependency. <details> <summary>Suggested change</summary> ```diff const allInvoices: Invoice[] = []; let startingAfter: string | undefined; - while (true) { + const MAX_PAGES = 100; // safety cap: 100 * 100 = 10k invoices + for (let page = 0; page < MAX_PAGES; page++) { const result = await client.invoices({ activationCode, limit: 100, ...(startingAfter && { startingAfter }), }); if (isServiceError(result)) { return result; } allInvoices.push(...result.invoices); if (!result.hasMore) { break; } const lastInvoice = result.invoices[result.invoices.length - 1]; if (!lastInvoice) { break; } + if (lastInvoice.id === startingAfter) { + // cursor isn't advancing — bail out to avoid an infinite loop + break; + } startingAfter = lastInvoice.id; } ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@packages/web/src/ee/features/lighthouse/actions.ts` around lines 190 - 214, The pagination loop in the invoice fetcher (allInvoices / startingAfter calling client.invoices and relying on result.hasMore and lastInvoice.id) must be bounded to avoid infinite loops; add a maxPages constant (e.g., MAX_INVOICE_PAGES) and a page counter inside the loop, increment it each iteration and break/return an error or log and stop when the counter exceeds the limit; update the while (true) to check the counter (or convert to a for loop) so the code stops even if hasMore stays true or the cursor doesn't advance, ensuring safe termination when fetching invoices via client.invoices. ``` </details> </blockquote></details> </blockquote></details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: Organization UI **Review profile**: CHILL **Plan**: Pro **Run ID**: `c60754b7-c2b8-4991-bea2-78cbf431dc1d` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 791496c533ee7f2b6a890160f696503cab873c2d and c725e3a43d0933ac29d7806652f4d89c719d2a98. </details> <details> <summary>📒 Files selected for processing (127)</summary> * `.env.development` * `CHANGELOG.md` * `docs/api-reference/sourcebot-public.openapi.json` * `docs/docs.json` * `docs/docs/billing.mdx` * `docs/docs/license-key.mdx` * `packages/backend/src/__mocks__/prisma.ts` * `packages/backend/src/api.ts` * `packages/backend/src/ee/accountPermissionSyncer.ts` * `packages/backend/src/ee/repoPermissionSyncer.ts` * `packages/backend/src/ee/syncSearchContexts.test.ts` * `packages/backend/src/ee/syncSearchContexts.ts` * `packages/backend/src/entitlements.ts` * `packages/backend/src/github.ts` * `packages/backend/src/index.ts` * `packages/backend/src/prisma.ts` * `packages/backend/src/utils.ts` * `packages/backend/vitest.config.ts` * `packages/db/prisma/migrations/20260417011834_add_license_table/migration.sql` * `packages/db/prisma/migrations/20260417224042_remove_guest_org_role/migration.sql` * `packages/db/prisma/migrations/20260418213423_add_billing_details_to_license/migration.sql` * `packages/db/prisma/migrations/20260421184633_add_cancel_at_to_license/migration.sql` * `packages/db/prisma/migrations/20260422203048_add_trial_fields/migration.sql` * `packages/db/prisma/migrations/20260422204809_add_has_payment_method/migration.sql` * `packages/db/prisma/schema.prisma` * `packages/shared/src/constants.ts` * `packages/shared/src/crypto.ts` * `packages/shared/src/entitlements.test.ts` * `packages/shared/src/entitlements.ts` * `packages/shared/src/env.server.ts` * `packages/shared/src/index.server.ts` * `packages/shared/src/types.ts` * `packages/shared/vitest.config.ts` * `packages/web/src/__mocks__/prisma.ts` * `packages/web/src/actions.ts` * `packages/web/src/app/(app)/@sidebar/components/defaultSidebar/index.tsx` * `packages/web/src/app/(app)/@sidebar/components/settingsSidebar/header.tsx` * `packages/web/src/app/(app)/@sidebar/components/settingsSidebar/index.tsx` * `packages/web/src/app/(app)/chat/[id]/page.tsx` * `packages/web/src/app/(app)/components/banners/actions.ts` * `packages/web/src/app/(app)/components/banners/bannerResolver.test.ts` * `packages/web/src/app/(app)/components/banners/bannerResolver.tsx` * `packages/web/src/app/(app)/components/banners/bannerShell.tsx` * `packages/web/src/app/(app)/components/banners/bannerSlot.tsx` * `packages/web/src/app/(app)/components/banners/invoicePastDueBanner.tsx` * `packages/web/src/app/(app)/components/banners/licenseExpiredBanner.tsx` * `packages/web/src/app/(app)/components/banners/licenseExpiryHeadsUpBanner.tsx` * `packages/web/src/app/(app)/components/banners/openBillingPortalButton.tsx` * `packages/web/src/app/(app)/components/banners/permissionSyncBanner.tsx` * `packages/web/src/app/(app)/components/banners/refreshLicenseButton.tsx` * `packages/web/src/app/(app)/components/banners/servicePingFailedBanner.tsx` * `packages/web/src/app/(app)/components/banners/trialBanner.tsx` * `packages/web/src/app/(app)/components/banners/types.ts` * `packages/web/src/app/(app)/layout.tsx` * `packages/web/src/app/(app)/repos/[id]/page.tsx` * `packages/web/src/app/(app)/settings/analytics/page.tsx` * `packages/web/src/app/(app)/settings/components/settingsCard.tsx` * `packages/web/src/app/(app)/settings/layout.tsx` * `packages/web/src/app/(app)/settings/license/activationCodeCard.tsx` * `packages/web/src/app/(app)/settings/license/currentPlanCard.tsx` * `packages/web/src/app/(app)/settings/license/offlineLicenseCard.tsx` * `packages/web/src/app/(app)/settings/license/page.tsx` * `packages/web/src/app/(app)/settings/license/planActionsMenu.tsx` * `packages/web/src/app/(app)/settings/license/recentInvoicesCard.tsx` * `packages/web/src/app/(app)/settings/linked-accounts/page.tsx` * `packages/web/src/app/(app)/settings/members/components/inviteMemberCard.tsx` * `packages/web/src/app/(app)/settings/members/components/invitesList.tsx` * `packages/web/src/app/(app)/settings/members/components/membersList.tsx` * `packages/web/src/app/(app)/settings/members/components/requestsList.tsx` * `packages/web/src/app/(app)/settings/members/page.tsx` * `packages/web/src/app/api/(server)/ee/.well-known/oauth-authorization-server/route.ts` * `packages/web/src/app/api/(server)/ee/.well-known/oauth-protected-resource/[...path]/route.ts` * `packages/web/src/app/api/(server)/ee/audit/route.ts` * `packages/web/src/app/api/(server)/ee/chat/[chatId]/searchMembers/route.ts` * `packages/web/src/app/api/(server)/ee/oauth/register/route.ts` * `packages/web/src/app/api/(server)/ee/oauth/revoke/route.ts` * `packages/web/src/app/api/(server)/ee/oauth/token/route.ts` * `packages/web/src/app/api/(server)/ee/permissionSyncStatus/api.ts` * `packages/web/src/app/api/(server)/ee/user/route.ts` * `packages/web/src/app/api/(server)/ee/users/route.ts` * `packages/web/src/app/api/(server)/mcp/route.ts` * `packages/web/src/app/api/(server)/repos/listReposApi.ts` * `packages/web/src/app/components/anonymousAccessToggle.tsx` * `packages/web/src/app/components/organizationAccessSettings.tsx` * `packages/web/src/app/invite/actions.ts` * `packages/web/src/app/invite/page.tsx` * `packages/web/src/app/layout.tsx` * `packages/web/src/app/login/page.tsx` * `packages/web/src/app/oauth/authorize/page.tsx` * `packages/web/src/app/onboard/page.tsx` * `packages/web/src/app/signup/page.tsx` * `packages/web/src/auth.ts` * `packages/web/src/ee/features/analytics/actions.ts` * `packages/web/src/ee/features/audit/actions.ts` * `packages/web/src/ee/features/audit/audit.ts` * `packages/web/src/ee/features/audit/auditService.ts` * `packages/web/src/ee/features/audit/factory.ts` * `packages/web/src/ee/features/audit/mockAuditService.ts` * `packages/web/src/ee/features/audit/types.ts` * `packages/web/src/ee/features/lighthouse/CLAUDE.md` * `packages/web/src/ee/features/lighthouse/actions.ts` * `packages/web/src/ee/features/lighthouse/client.ts` * `packages/web/src/ee/features/lighthouse/servicePing.ts` * `packages/web/src/ee/features/lighthouse/types.ts` * `packages/web/src/ee/features/sso/actions.ts` * `packages/web/src/ee/features/sso/sso.ts` * `packages/web/src/ee/features/userManagement/actions.ts` * `packages/web/src/features/chat/actions.ts` * `packages/web/src/features/git/getFileSourceApi.ts` * `packages/web/src/features/git/getTreeApi.ts` * `packages/web/src/features/mcp/askCodebase.ts` * `packages/web/src/features/search/searchApi.ts` * `packages/web/src/features/userManagement/actions.ts` * `packages/web/src/initialize.ts` * `packages/web/src/lib/authUtils.ts` * `packages/web/src/lib/constants.ts` * `packages/web/src/lib/entitlements.test.ts` * `packages/web/src/lib/entitlements.ts` * `packages/web/src/lib/errorCodes.ts` * `packages/web/src/lib/identityProviders.ts` * `packages/web/src/lib/utils.ts` * `packages/web/src/middleware/authenticatedPage.tsx` * `packages/web/src/middleware/withAuth.test.ts` * `packages/web/src/middleware/withAuth.ts` * `packages/web/src/middleware/withMinimumOrgRole.ts` * `packages/web/src/openapi/publicApiSchemas.ts` * `packages/web/src/prisma.ts` </details> <details> <summary>💤 Files with no reviewable changes (7)</summary> * packages/web/src/app/(app)/settings/linked-accounts/page.tsx * packages/shared/src/constants.ts * packages/web/src/middleware/withMinimumOrgRole.ts * packages/web/src/ee/features/audit/factory.ts * packages/web/src/lib/constants.ts * packages/web/src/ee/features/audit/mockAuditService.ts * packages/web/src/ee/features/audit/auditService.ts </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
Offline license expired banner (owner):

Offline license expired banner (member):

Online license expired banner (owner):

Online license expired banner (member):

Permission sync banner:

Expiry heads up banner:

Invoice past due banner:

License stale warning banner:

License stale error banner (owner):

License stale error banner (member):

Trial banner (payment method added):

Trial banner (payment method not added):

Summary by CodeRabbit
New Features
Bug Fixes
Documentation